Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 4 | Implement shell tools (cat, ls, wc) in Python by HassanOHOsman · Pull Request #285 · CodeYourFuture/Module-Tools

HassanOHOsman · 2025-12-05T21:56:10Z

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

This PR provides implementations of shell tools: cat, ls, and wc in Python.

… is blak and if so removes it

…for -n & -b options

…ementation

…when -a option not added by end user

…ent to diplay in one line with spaces in between

…al else

…ual ls with -1 flag

…ts and updating total

…ile_path instead

OracPrime

You've made it very hard to review - that in itself is a problem. Don't commit individual file changes, commit self-consistent changes to your code. I should be able to pull any commit and it should work. I should be able to look at a single commit and see what the big change is.

At least one comment needs addressing - the double space in the ending.

OracPrime · 2026-01-14T22:40:13Z

implement-shell-tools/cat/cat.py

+
+        for line in lines:
+            if args.n:
+                print(f"{line_number} {line}")


If the function of software is something else, then the formatting of the output may be less important. But the purpose of this software is to present information to the user, so it needed sorting

OracPrime · 2026-01-14T22:45:24Z

implement-shell-tools/ls/ls.py

                print(item)
            else:
-                print(item, end=" ")
+                print(item, end="  ")


Why two spaces? Should it be \t? I don't actually know, but I think you should find out :)

replaced the double spacing with /t. Tested it and worked. Thank you!

OracPrime · 2026-01-14T22:46:31Z

implement-shell-tools/wc/wc.py



+def process_file(file_path):
+    global total_lines, total_words, total_characters, file_count


global is rarely a good word to see in source code... what's it doing here?

the goal of having it so that total_lines, total_words, total_characters, file_count can be used inside my helper function otherwise, an error would've been thrown for them not being defined locally. Anyway, I am looking for alternatives in the meantime.

A function can return more than one value. You can define them in the function and them have them all on the return statement. For example

def fun():
return "stuff", 20

s, x = fun()
print(s)
print(x)

OracPrime · 2026-01-14T22:48:10Z

implement-shell-tools/wc/wc.py


    if args.line:
-        print(f"{stats['lines']} {path}")
+        print(f"{stats['lines']} {file_path}}")


Unless I'm seeing some distorted github view of reality, it looks like you are committing individual file changes as one commit each. You don't want to do that. Each commit should be a group of file changes which is logically consistent.

My bad, I thought it frequent committing is the way. I will aim to group my commits moving forward. Thank you!

Frequent commits are good, but every commit should be a logical change: the commit description should describe the group of changes being made, but it's all the changes need for a particular step forward, not file by file

OracPrime · 2026-01-14T22:53:44Z

implement-shell-tools/ls/ls.py

+
+parser.add_argument("-1", "--one", dest="one", help="Output one entry per line", action="store_true")
+parser.add_argument("-a", help="List all files & directories, including hidden ones", action="store_true")
+parser.add_argument("paths", nargs="*", default=["."], help="The file path to read from")


Arg is paths, comment is path. That will confuse people.

Reworded the confusing bit. Hope it's good now.

…ct ls do when listing files in a dir

…own and get rid of code no longer in use

OracPrime

Nice work. I think you're done with this one now!

HassanOHOsman added 30 commits December 1, 2025 13:24

create cat.py file to write our implementation on it

4675246

import argparse library

95cfc32

define program meta data

530a234

define the -n option

2fc8e5d

define the -b option/flag

73369ea

define the path / file to read from

3bb2ce9

read the command line argument during runtime

652c027

open the file/path for reading and laod its content

1f95e09

added () to resolve buggy code

9de5129

splitting file content by end of line

42b98a5

after splitting the file content into lines checking if the last line…

f1444f9

… is blak and if so removes it

handle -n flag case

4da7430

handles the -b option case and it's 2 scenarios

414ebd7

if no flag is selected print the content

a0daa76

corrected the increment to follow python syntax

cd5256b

corrected the logic for when no flag was selected

b1bf214

adding action="store_true" so that the program doesn't expect values …

d0bb387

…for -n & -b options

corrected code to follow python syntax

80e3909

correct logic for options to further mimic cat tool behaviour

3ce06c4

corrected logic to handle several files/path arguments

e93d1f9

remove js implementation which used to guide me to do the python impl…

f1b5a23

…ementation

create ls.py file to implement ls tool in python in it

f3e7e68

add ls implementation in js to guide me when implemening in js

1cbdfd4

define ls program meta data (name, options, paths etc)

719bf23

read the comman line durign runtime

2987f4b

handling one path with ls

5b493c9

import os library so that you can list files & dir inside a working dir

6c8a567

listing the files & dirs inside the working dir

83e99f3

alogic in which the hidden files & dirs are filtered out (not shown) …

1d1ac0a

…when -a option not added by end user

logic added to handle one 1/one optition is added and if not all cont…

9a40bd6

…ent to diplay in one line with spaces in between

HassanOHOsman added 13 commits January 14, 2026 21:18

improving the quality of code for -a flag code block and adding a fin…

a6642bb

…al else

simplify the for loop

6a5281e

adding double spaces for the -one flag to match the format of the act…

683fc0c

…ual ls with -1 flag

adding sorting method so that it matches the actual ls output order

b95cf8e

discarding the redantant commented js implementation

e3a7ce3

setting up a helper function that handles reading files, printing sta…

82a6800

…ts and updating total

moving code into the helper function proccess_file

e9f2a54

update the default paremeter of the helper function process_file to f…

9f5649f

…ile_path instead

moving code inside helper function process_file

14fca64

get rid of repetetive code

140467f

edit code for when path is dir

088df21

change parameter to file_path to get rid of bug

4b2801f

getting rid of all extra }

6551026

HassanOHOsman added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jan 14, 2026

OracPrime reviewed Jan 14, 2026

View reviewed changes

OracPrime added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jan 14, 2026

HassanOHOsman added 4 commits January 14, 2026 23:29

replace the double spacing with \t to cause the "double spacing" effe…

b1b1669

…ct ls do when listing files in a dir

change logic to elminate the need for using keyword "global"

5158913

move total before helper function process_file otherwise error is thr…

92929e0

…own and get rid of code no longer in use

improve the readability to eliminate misinterpration

85622de

HassanOHOsman added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jan 15, 2026

OracPrime approved these changes Jan 15, 2026

View reviewed changes

HassanOHOsman removed the Reviewed Volunteer to add when completing a review with trainee action still to take. label Jan 15, 2026



		def process_file(file_path):
		global total_lines, total_words, total_characters, file_count

Uh oh!

Conversation

HassanOHOsman commented Dec 5, 2025

Uh oh!

OracPrime left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OracPrime left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants